Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debuginfod Testing & fixes: 3rd times the charm? #87676

Merged

Conversation

kevinfrei
Copy link
Contributor

I believe I've got the tests properly configured to only run on Linux x86(_64), as I don't have a Linux AArch64/Arm device to diagnose what's going wrong with the tests (I suspect there's some issue with generating .note.gnu.build-id sections...)

The actual code fixes have now been reviewed 3 times: #79181 (moved shell tests to API tests), #85693 (Changed some of the testing infra), and #86812 (didn't get the tests configured quite right). The Debuginfod integration for symbol acquisition in LLDB now works with the executable and debuginfo Debuginfod network requests working properly for normal, objcopy --only-keep-debug stripped, split-dwarf, and objcopy --only-keep-debug stripped plus split-dwarf symbols/binaries.

The reasons for the multiple attempts have been tests on platforms I don't have access to (Linux AArch64/Arm + MacOS x86_64). I believe I've got the tests properly disabled for everything except for Linux x86(_64) now. I've built & tested on MacOS AArch64 and Linux x86_64.

@kevinfrei kevinfrei force-pushed the debuginfod-testing-3rd-times-the-charm branch from be08033 to f2259b4 Compare April 4, 2024 18:39
@kevinfrei kevinfrei marked this pull request as ready for review April 4, 2024 18:42
@llvmbot llvmbot added the lldb label Apr 4, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-lldb

Author: Kevin Frei (kevinfrei)

Changes

I believe I've got the tests properly configured to only run on Linux x86(_64), as I don't have a Linux AArch64/Arm device to diagnose what's going wrong with the tests (I suspect there's some issue with generating .note.gnu.build-id sections...)

The actual code fixes have now been reviewed 3 times: #79181 (moved shell tests to API tests), #85693 (Changed some of the testing infra), and #86812 (didn't get the tests configured quite right). The Debuginfod integration for symbol acquisition in LLDB now works with the executable and debuginfo Debuginfod network requests working properly for normal, objcopy --only-keep-debug stripped, split-dwarf, and objcopy --only-keep-debug stripped plus split-dwarf symbols/binaries.

The reasons for the multiple attempts have been tests on platforms I don't have access to (Linux AArch64/Arm + MacOS x86_64). I believe I've got the tests properly disabled for everything except for Linux x86(_64) now. I've built & tested on MacOS AArch64 and Linux x86_64.


Patch is 25.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87676.diff

10 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/make/Makefile.rules (+25-1)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+25-13)
  • (modified) lldb/source/Plugins/SymbolLocator/CMakeLists.txt (+6-1)
  • (modified) lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp (+27-2)
  • (added) lldb/test/API/debuginfod/Normal/Makefile (+19)
  • (added) lldb/test/API/debuginfod/Normal/TestDebuginfod.py (+183)
  • (added) lldb/test/API/debuginfod/Normal/main.c (+7)
  • (added) lldb/test/API/debuginfod/SplitDWARF/Makefile (+23)
  • (added) lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py (+193)
  • (added) lldb/test/API/debuginfod/SplitDWARF/main.c (+7)
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index bfd249ccd43f2e..ee8793fa1b3029 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../
 #
 # GNUWin32 uname gives "windows32" or "server version windows32" while
 # some versions of MSYS uname return "MSYS_NT*", but most environments
-# standardize on "Windows_NT", so we'll make it consistent here. 
+# standardize on "Windows_NT", so we'll make it consistent here.
 # When running tests from Visual Studio, the environment variable isn't
 # inherited all the way down to the process spawned for make.
 #----------------------------------------------------------------------
@@ -210,6 +210,12 @@ else
 	ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
 		DSYM = $(EXE).debug
 	endif
+
+	ifeq "$(MAKE_DWP)" "YES"
+		MAKE_DWO := YES
+		DWP_NAME = $(EXE).dwp
+		DYLIB_DWP_NAME = $(DYLIB_NAME).dwp
+	endif
 endif
 
 LIMIT_DEBUG_INFO_FLAGS =
@@ -357,6 +363,7 @@ ifneq "$(OS)" "Darwin"
 
 	OBJCOPY ?= $(call replace_cc_with,objcopy)
 	ARCHIVER ?= $(call replace_cc_with,ar)
+	DWP ?= $(call replace_cc_with,dwp)
 	override AR = $(ARCHIVER)
 endif
 
@@ -527,6 +534,10 @@ ifneq "$(CXX)" ""
 	endif
 endif
 
+ifeq "$(GEN_GNU_BUILD_ID)" "YES"
+	LDFLAGS += -Wl,--build-id
+endif
+
 #----------------------------------------------------------------------
 # DYLIB_ONLY variable can be used to skip the building of a.out.
 # See the sections below regarding dSYM file as well as the building of
@@ -565,10 +576,17 @@ else
 endif
 else
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+	cp "$(EXE)" "$(EXE).unstripped"
+endif
 	$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
 	$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
 endif
+ifeq "$(MAKE_DWP)" "YES"
+	$(DWP) -o "$(DWP_NAME)" $(DWOS)
 endif
+endif
+
 
 #----------------------------------------------------------------------
 # Make the dylib
@@ -610,9 +628,15 @@ endif
 else
 	$(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)"
 ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+	ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+	cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped"
+	endif
 	$(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug"
 	$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)"
 endif
+ifeq "$(MAKE_DWP)" "YES"
+	$(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS)
+endif
 endif
 
 #----------------------------------------------------------------------
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 49f13d2c89e380..dafdf241f9db0c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4378,26 +4378,38 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
     FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
     ModuleSpec module_spec;
     module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
+    FileSpec dwp_filespec;
     for (const auto &symfile : symfiles.files()) {
       module_spec.GetSymbolFileSpec() =
           FileSpec(symfile.GetPath() + ".dwp", symfile.GetPathStyle());
       LLDB_LOG(log, "Searching for DWP using: \"{0}\"",
                module_spec.GetSymbolFileSpec());
-      FileSpec dwp_filespec =
+      dwp_filespec =
           PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
       if (FileSystem::Instance().Exists(dwp_filespec)) {
-        LLDB_LOG(log, "Found DWP file: \"{0}\"", dwp_filespec);
-        DataBufferSP dwp_file_data_sp;
-        lldb::offset_t dwp_file_data_offset = 0;
-        ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
-            GetObjectFile()->GetModule(), &dwp_filespec, 0,
-            FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
-            dwp_file_data_offset);
-        if (dwp_obj_file) {
-          m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
-              *this, dwp_obj_file, DIERef::k_file_index_mask);
-          break;
-        }
+        break;
+      }
+    }
+    if (!FileSystem::Instance().Exists(dwp_filespec)) {
+      LLDB_LOG(log, "No DWP file found locally");
+      // Fill in the UUID for the module we're trying to match for, so we can
+      // find the correct DWP file, as the Debuginfod plugin uses *only* this
+      // data to correctly match the DWP file with the binary.
+      module_spec.GetUUID() = m_objfile_sp->GetUUID();
+      dwp_filespec =
+          PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
+    }
+    if (FileSystem::Instance().Exists(dwp_filespec)) {
+      LLDB_LOG(log, "Found DWP file: \"{0}\"", dwp_filespec);
+      DataBufferSP dwp_file_data_sp;
+      lldb::offset_t dwp_file_data_offset = 0;
+      ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+          GetObjectFile()->GetModule(), &dwp_filespec, 0,
+          FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
+          dwp_file_data_offset);
+      if (dwp_obj_file) {
+        m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
+            *this, dwp_obj_file, DIERef::k_file_index_mask);
       }
     }
     if (!m_dwp_symfile) {
diff --git a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
index ca969626f4ffc4..3367022639ab85 100644
--- a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
+++ b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt
@@ -1,5 +1,10 @@
+# Order matters here: the first symbol locator prevents further searching.
+# For DWARF binaries that are both stripped and split, the Default plugin
+# will return the stripped binary when asked for the ObjectFile, which then
+# prevents an unstripped binary from being requested from the Debuginfod
+# provider.
+add_subdirectory(Debuginfod)
 add_subdirectory(Default)
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(DebugSymbols)
 endif()
-add_subdirectory(Debuginfod)
diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
index b5fe35d71032a8..f296e655cc466d 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -44,6 +44,24 @@ llvm::StringRef SymbolVendorELF::GetPluginDescriptionStatic() {
          "executables.";
 }
 
+// If this is needed elsewhere, it can be exported/moved.
+static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp,
+                            const FileSpec &file_spec) {
+  DataBufferSP dwp_file_data_sp;
+  lldb::offset_t dwp_file_data_offset = 0;
+  // Try to create an ObjectFile from the file_spec.
+  ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+      module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
+      dwp_file_data_sp, dwp_file_data_offset);
+  // The presence of a debug_cu_index section is the key identifying feature of
+  // a DWP file. Make sure we don't fill in the section list on dwp_obj_file
+  // (by calling GetSectionList(false)) as this function could be called before
+  // we may have all the symbol files collected and available.
+  return dwp_obj_file && ObjectFileELF::classof(dwp_obj_file.get()) &&
+         dwp_obj_file->GetSectionList(false)->FindSectionByType(
+             eSectionTypeDWARFDebugCuIndex, false);
+}
+
 // CreateInstance
 //
 // Platforms can register a callback to use when creating symbol vendors to
@@ -87,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp,
   FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
   FileSpec dsym_fspec =
       PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
-  if (!dsym_fspec)
-    return nullptr;
+  if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) {
+    // If we have a stripped binary or if we got a DWP file, we should prefer
+    // symbols in the executable acquired through a plugin.
+    ModuleSpec unstripped_spec =
+        PluginManager::LocateExecutableObjectFile(module_spec);
+    if (!unstripped_spec)
+      return nullptr;
+    dsym_fspec = unstripped_spec.GetFileSpec();
+  }
 
   DataBufferSP dsym_file_data_sp;
   lldb::offset_t dsym_file_data_offset = 0;
diff --git a/lldb/test/API/debuginfod/Normal/Makefile b/lldb/test/API/debuginfod/Normal/Makefile
new file mode 100644
index 00000000000000..54bd7adae241f5
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/Makefile
@@ -0,0 +1,19 @@
+C_SOURCES := main.c
+
+# For normal (non DWP) Debuginfod tests, we need:
+
+# * The full binary: a.out.unstripped
+#   Produced by Makefile.rules with SAVE_FULL_DEBUG_BINARY set to YES and
+#   SPLIT_DEBUG_SYMBOLS set to YES
+
+# * The stripped binary (a.out)
+#   Produced by Makefile.rules with SPLIT_DEBUG_SYMBOLS set to YES
+
+# * The 'only-keep-debug' binary (a.out.debug)
+#   Produced below
+
+SPLIT_DEBUG_SYMBOLS := YES
+SAVE_FULL_DEBUG_BINARY := YES
+GEN_GNU_BUILD_ID := YES
+
+include Makefile.rules
diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
new file mode 100644
index 00000000000000..f1be1e705ee0cb
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -0,0 +1,183 @@
+import os
+import shutil
+import tempfile
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This one is for simple / no split-dwarf scenarios.
+
+For no-split-dwarf scenarios, there are 2 variations:
+1 - A stripped binary with it's corresponding unstripped binary:
+2 - A stripped binary with a corresponding --only-keep-debug symbols file
+"""
+
+
+# It looks like Linux-AArch64 doesn't support build-id's on the LLDB builtbots
+class DebugInfodTests(TestBase):
+    # No need to try every flavor of debug inf.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_normal_no_symbols(self):
+        """
+        Validate behavior with no symbols or symbol locator.
+        ('baseline negative' behavior)
+        """
+        test_root = self.config_test(["a.out"])
+        self.try_breakpoint(False)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_normal_default(self):
+        """
+        Validate behavior with symbols, but no symbol locator.
+        ('baseline positive' behavior)
+        """
+        test_root = self.config_test(["a.out", "a.out.debug"])
+        self.try_breakpoint(True)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_debuginfod_symbols(self):
+        """
+        Test behavior with the full binary available from Debuginfod as
+        'debuginfo' from the plug-in.
+        """
+        test_root = self.config_test(["a.out"], "a.out.unstripped")
+        self.try_breakpoint(True)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_debuginfod_executable(self):
+        """
+        Test behavior with the full binary available from Debuginfod as
+        'executable' from the plug-in.
+        """
+        test_root = self.config_test(["a.out"], None, "a.out.unstripped")
+        self.try_breakpoint(True)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_debuginfod_okd_symbols(self):
+        """
+        Test behavior with the 'only-keep-debug' symbols available from Debuginfod.
+        """
+        test_root = self.config_test(["a.out"], "a.out.debug")
+        self.try_breakpoint(True)
+
+    def try_breakpoint(self, should_have_loc):
+        """
+        This function creates a target from self.aout, sets a function-name
+        breakpoint, and checks to see if we have a file/line location,
+        as a way to validate that the symbols have been loaded.
+        should_have_loc specifies if we're testing that symbols have or
+        haven't been loaded.
+        """
+        target = self.dbg.CreateTarget(self.aout)
+        self.assertTrue(target and target.IsValid(), "Target is valid")
+
+        bp = target.BreakpointCreateByName("func")
+        self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid")
+        self.assertEqual(bp.GetNumLocations(), 1)
+
+        loc = bp.GetLocationAtIndex(0)
+        self.assertTrue(loc and loc.IsValid(), "Location is valid")
+        addr = loc.GetAddress()
+        self.assertTrue(addr and addr.IsValid(), "Loc address is valid")
+        line_entry = addr.GetLineEntry()
+        self.assertEqual(
+            should_have_loc,
+            line_entry != None and line_entry.IsValid(),
+            "Loc line entry is valid",
+        )
+        if should_have_loc:
+            self.assertEqual(line_entry.GetLine(), 4)
+            self.assertEqual(
+                line_entry.GetFileSpec().GetFilename(),
+                self.main_source_file.GetFilename(),
+            )
+        self.dbg.DeleteTarget(target)
+        shutil.rmtree(self.tmp_dir)
+
+    def config_test(self, local_files, debuginfo=None, executable=None):
+        """
+        Set up a test with local_files[] copied to a different location
+        so that we control which files are, or are not, found in the file system.
+        Also, create a stand-alone file-system 'hosted' debuginfod server with the
+        provided debuginfo and executable files (if they exist)
+
+        Make the filesystem look like:
+
+        /tmp/<tmpdir>/test/[local_files]
+
+        /tmp/<tmpdir>/cache (for lldb to use as a temp cache)
+
+        /tmp/<tmpdir>/buildid/<uuid>/executable -> <executable>
+        /tmp/<tmpdir>/buildid/<uuid>/debuginfo -> <debuginfo>
+        Returns the /tmp/<tmpdir> path
+        """
+
+        self.build()
+
+        uuid = self.getUUID("a.out")
+        if not uuid:
+            self.fail("Could not get UUID for a.out")
+            return
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.tmp_dir = tempfile.mkdtemp()
+        test_dir = os.path.join(self.tmp_dir, "test")
+        os.makedirs(test_dir)
+
+        self.aout = ""
+        # Copy the files used by the test:
+        for f in local_files:
+            shutil.copy(self.getBuildArtifact(f), test_dir)
+            # The first item is the binary to be used for the test
+            if self.aout == "":
+                self.aout = os.path.join(test_dir, f)
+
+        use_debuginfod = debuginfo != None or executable != None
+
+        # Populated the 'file://... mocked' Debuginfod server:
+        if use_debuginfod:
+            os.makedirs(os.path.join(self.tmp_dir, "cache"))
+            uuid_dir = os.path.join(self.tmp_dir, "buildid", uuid)
+            os.makedirs(uuid_dir)
+            if debuginfo:
+                shutil.copy(
+                    self.getBuildArtifact(debuginfo),
+                    os.path.join(uuid_dir, "debuginfo"),
+                )
+            if executable:
+                shutil.copy(
+                    self.getBuildArtifact(executable),
+                    os.path.join(uuid_dir, "executable"),
+                )
+
+        # Configure LLDB for the test:
+        self.runCmd(
+            "settings set symbols.enable-external-lookup %s"
+            % str(use_debuginfod).lower()
+        )
+        self.runCmd("settings clear plugin.symbol-locator.debuginfod.server-urls")
+        if use_debuginfod:
+            self.runCmd(
+                "settings set plugin.symbol-locator.debuginfod.cache-path %s/cache"
+                % self.tmp_dir
+            )
+            self.runCmd(
+                "settings insert-before plugin.symbol-locator.debuginfod.server-urls 0 file://%s"
+                % self.tmp_dir
+            )
+
+    def getUUID(self, filename):
+        try:
+            target = self.dbg.CreateTarget(self.getBuildArtifact(filename))
+            module = target.GetModuleAtIndex(0)
+            uuid = module.GetUUIDString().replace("-", "").lower()
+            self.dbg.DeleteTarget(target)
+            return uuid if len(uuid) == 40 else None
+        except:
+            return None
diff --git a/lldb/test/API/debuginfod/Normal/main.c b/lldb/test/API/debuginfod/Normal/main.c
new file mode 100644
index 00000000000000..4c7184609b4536
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/main.c
@@ -0,0 +1,7 @@
+// This is a dump little pair of test files
+
+int func(int argc, const char *argv[]) {
+  return (argc + 1) * (argv[argc][0] + 2);
+}
+
+int main(int argc, const char *argv[]) { return func(0, argv); }
diff --git a/lldb/test/API/debuginfod/SplitDWARF/Makefile b/lldb/test/API/debuginfod/SplitDWARF/Makefile
new file mode 100644
index 00000000000000..3ab9a969e5a447
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/Makefile
@@ -0,0 +1,23 @@
+C_SOURCES := main.c
+
+# For split-dwarf Debuginfod tests, we need:
+
+# * A .DWP file (a.out.dwp)
+#   Produced by Makefile.rules with MAKE_DWP set to YES
+
+# * The "full" binary (missing things that live in .dwo's) (a.out.unstripped)
+#   Produced by Makefile.rules with SAVE_FULL_DEBUG_BINARY set to YES and
+#   SPLIT_DEBUG_SYMBOLS set to YES
+
+# * The stripped binary (a.out)
+#   Produced by Makefile.rules
+
+# * The 'only-keep-debug' binary (a.out.debug)
+#   Produced below
+
+MAKE_DWP := YES
+SPLIT_DEBUG_SYMBOLS := YES
+SAVE_FULL_DEBUG_BINARY := YES
+GEN_GNU_BUILD_ID := YES
+
+include Makefile.rules
diff --git a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
new file mode 100644
index 00000000000000..fec2fa1132d776
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
@@ -0,0 +1,193 @@
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+"""
+import os
+import shutil
+import tempfile
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This file is for split-dwarf (dwp) scenarios.
+
+1 - A split binary target with it's corresponding DWP file
+2 - A stripped, split binary target with an unstripped binary and a DWP file
+3 - A stripped, split binary target with an --only-keep-debug symbols file and a DWP file
+"""
+
+
+# It looks like Linux-AArch64 doesn't support build-id's on the LLDB builtbots
+class DebugInfodDWPTests(TestBase):
+    # No need to try every flavor of debug inf.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_normal_stripped(self):
+        """
+        Validate behavior with a stripped binary, no symbols or symbol locator.
+        """
+        self.config_test(["a.out"])
+        self.try_breakpoint(False)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_normal_stripped_split_with_dwp(self):
+        """
+        Validate behavior with symbols, but no symbol locator.
+        """
+        self.config_test(["a.out", "a.out.debug", "a.out.dwp"])
+        self.try_breakpoint(True)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_normal_stripped_only_dwp(self):
+        """
+        Validate behavior *with* dwp symbols only, but missing other symbols,
+        but no symbol locator. This shouldn't work: without the other symbols
+        DWO's appear mostly useless.
+        """
+        self.config_test(["a.out", "a.out.dwp"])
+        self.try_breakpoint(False)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_debuginfod_dwp_from_service(self):
+        """
+        Test behavior with the unstripped binary, and DWP from the service.
+        """
+        self.config_test(["a.out.debug"], "a.out.dwp")
+        self.try_breakpoint(True)
+
+    @skipIf(oslist=no_match(["linux"]), archs=no_match(["i386", "x86_64"]))
+    def test_debuginfod_both_symfiles_from_service...
[truncated]

@clayborg clayborg merged commit d6713ad into llvm:main Apr 4, 2024
7 checks passed
@rastogishubham
Copy link
Contributor

Hi @kevinfrei it seems like this patch broke greendragon

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/812/

@gulfemsavrun
Copy link
Contributor

gulfemsavrun commented Apr 4, 2024

It also broke our toolchain builders:
https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8751560900790274577/overview

FAIL: lldb-api :: debuginfod/Normal/TestDebuginfod.py (239 of 2489)
******************** TEST 'lldb-api :: debuginfod/Normal/TestDebuginfod.py' FAILED ********************
Script:
--
/b/s/w/ir/x/w/lldb_install/python3/bin/python3 /b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/b/s/w/ir/x/w/cipd/bin/llvm-ar --env OBJCOPY=/b/s/w/ir/x/w/cipd/bin/llvm-objcopy --env LLVM_LIBS_DIR=/b/s/w/ir/x/w/llvm_build/./lib --env LLVM_INCLUDE_DIR=/b/s/w/ir/x/w/llvm_build/include --env LLVM_TOOLS_DIR=/b/s/w/ir/x/w/llvm_build/./bin --libcxx-include-dir /b/s/w/ir/x/w/llvm_build/include/c++/v1 --libcxx-include-target-dir /b/s/w/ir/x/w/llvm_build/include/x86_64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /b/s/w/ir/x/w/llvm_build/./lib/x86_64-unknown-linux-gnu --arch x86_64 --build-dir /b/s/w/ir/x/w/llvm_build/lldb-test-build.noindex --lldb-module-cache-dir /b/s/w/ir/x/w/llvm_build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /b/s/w/ir/x/w/llvm_build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /b/s/w/ir/x/w/llvm_build/./bin/lldb --compiler /b/s/w/ir/x/w/llvm_build/./bin/clang --dsymutil /b/s/w/ir/x/w/llvm_build/./bin/dsymutil --llvm-tools-dir /b/s/w/ir/x/w/llvm_build/./bin --lldb-libs-dir /b/s/w/ir/x/w/llvm_build/./lib --skip-category=pexpect /b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/debuginfod/Normal -p TestDebuginfod.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 19.0.0git (https://llvm.googlesource.com/a/llvm-project revision 8a0bfe490592de3df28d82c5dd69956e43c20f1d)
  clang revision 8a0bfe490592de3df28d82c5dd69956e43c20f1d
  llvm revision 8a0bfe490592de3df28d82c5dd69956e43c20f1d
Skipping the following test categories: ['pexpect', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
FAIL: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_debuginfod_executable (TestDebuginfod.DebugInfodTests.test_debuginfod_executable)
FAIL: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_debuginfod_okd_symbols (TestDebuginfod.DebugInfodTests.test_debuginfod_okd_symbols)
FAIL: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_debuginfod_symbols (TestDebuginfod.DebugInfodTests.test_debuginfod_symbols)
FAIL: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_normal_default (TestDebuginfod.DebugInfodTests.test_normal_default)
FAIL: LLDB (/b/s/w/ir/x/w/llvm_build/bin/clang-x86_64) :: test_normal_no_symbols (TestDebuginfod.DebugInfodTests.test_normal_no_symbols)
======================================================================
FAIL: test_debuginfod_executable (TestDebuginfod.DebugInfodTests.test_debuginfod_executable)
   Test behavior with the full binary available from Debuginfod as
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/b/s/w/ir/x/w/llvm-llvm-project/lldb/packages/Python/lldbsuite/test/decorators.py", line 150, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/debuginfod/Normal/TestDebuginfod.py", line 59, in test_debuginfod_executable
    test_root = self.config_test(["a.out"], None, "a.out.unstripped")
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/b/s/w/ir/x/w/llvm-llvm-project/lldb/test/API/debuginfod/Normal/TestDebuginfod.py", line 126, in config_test
    self.fail("Could not get UUID for a.out")
AssertionError: Could not get UUID for a.out
Config=x86_64-/b/s/w/ir/x/w/llvm_build/bin/clang

https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket/8751560900790274577/+/u/lldb/test/stdout

rastogishubham added a commit that referenced this pull request Apr 4, 2024
This reverts commit d6713ad.

This changed was reverted because of greendragon failures such
as

Unresolved Tests (2):
  lldb-api :: debuginfod/Normal/TestDebuginfod.py
  lldb-api :: debuginfod/SplitDWARF/TestDebuginfodDWP.py
@rastogishubham
Copy link
Contributor

To make sure green dragon stays green, I reverted this change with ca55ee8

@DavidSpickett
Copy link
Collaborator

When I tried these on Arm, .note.gnu.build-id was not the issue, it was failures like those seen in the Fuschia logs.
/bin/sh: 1: ifeq: not found and /bin/sh: 1: /b/s/w/ir/x/w/llvm_build/bin/dwp: not found (plus some other tools).

Leave the tests disabled on Arm and I'll try them again once the current issues are resolved.

clayborg pushed a commit that referenced this pull request May 2, 2024
I'm taking yet another swing at getting these tests going, on the
hypothesis that the problems with buildbots & whatnot are because
they're not configured with CURL support, which I've confirmed would
cause the previous tests to fail. (I have no access to an ARM64 linux
system, but I did repro the failure on MacOS configured without CURL
support)

So, the only difference between this diff and
[previous](#85693)
[diffs](#87676) that have
already been approved is that I've added a condition to the tests to
only run if Debuginfod capabilities should be built into the binary. I
had done this for these tests when they were [Shell
tests](#79181) and not API
tests, but I couldn't find a direct analog in any API test, so I used
the "plugins" model used by the intel-pt tests as well.

---------

Co-authored-by: Kevin Frei <[email protected]>
clayborg pushed a commit that referenced this pull request May 22, 2024
Here we go with attempt number five. Again, no changes to the LLDB code
diff, which has been reviewed several times.

For the tests, I added a `@skipIfCurlSupportMissing` annotation so that
the Debuginfod mocked server stuff won't run, and I also disabled
non-Linux/FreeBSD hosts altogether, as they fail for platform reasons on
macOS and Windows. In addition, I updated the process for extracting the
GNU BuildID to no create a target, per some feedback on the previous
diff.

For reference, previous PR's (landed, backed out after the fact for
various reasons) #90622, #87676, #86812, #85693

---------

Co-authored-by: Kevin Frei <[email protected]>
clayborg pushed a commit that referenced this pull request Jul 8, 2024
This is the same diff I've put up at many times before. I've been trying
to add some brand new functionality to the LLDB test infrastucture
(create split-dwarf files!), and we all know that no good deed goes
unpunished. The last attempt was reverted because it didn't work on the
Fuchsia build.

There are no code differences between this and
[the](#90622)
[previous](#87676)
[four](#86812)
[diffs](#85693) landed &
reverted (due to testing infra failures). The only change in this one is
the way `dwp` is being identified in `Makefile.rules`.

Thanks to @petrhosek for helping me figure out how the fuchsia builders
are configured. I now prefer to use llvm-dwp and fall back to gnu's dwp
if the former isn't found. Hopefully this will work everywhere it needs
to.
JDevlieghere pushed a commit that referenced this pull request Jul 10, 2024
This is all the tests and fixes I've had percolating since my first
attempt at this in January. After 6 months of trying, I've given up on
adding the ability to test DWP files in LLDB API tests. I've left both
the tests (disabled) and the changes to Makefile.rules in place, in the
hopes that someone who can configure the build bots will be able to
enable the tests once a non-borked dwp tool is widely available.

Other than disabling the DWP tests, this continues to be the same diff
that I've tried to land and
[not](#90622)
[revert](#87676)
[five](#86812)
[times](#85693)
[before](#96802). There are a
couple of fixes that the testing exposed, and I've abandoned the DWP
tests because I want to get those fixes finally upstreamed, as without
them DebugInfoD is less useful.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
This is the same diff I've put up at many times before. I've been trying
to add some brand new functionality to the LLDB test infrastucture
(create split-dwarf files!), and we all know that no good deed goes
unpunished. The last attempt was reverted because it didn't work on the
Fuchsia build.

There are no code differences between this and
[the](llvm#90622)
[previous](llvm#87676)
[four](llvm#86812)
[diffs](llvm#85693) landed &
reverted (due to testing infra failures). The only change in this one is
the way `dwp` is being identified in `Makefile.rules`.

Thanks to @petrhosek for helping me figure out how the fuchsia builders
are configured. I now prefer to use llvm-dwp and fall back to gnu's dwp
if the former isn't found. Hopefully this will work everywhere it needs
to.
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…#98344)

This is all the tests and fixes I've had percolating since my first
attempt at this in January. After 6 months of trying, I've given up on
adding the ability to test DWP files in LLDB API tests. I've left both
the tests (disabled) and the changes to Makefile.rules in place, in the
hopes that someone who can configure the build bots will be able to
enable the tests once a non-borked dwp tool is widely available.

Other than disabling the DWP tests, this continues to be the same diff
that I've tried to land and
[not](llvm#90622)
[revert](llvm#87676)
[five](llvm#86812)
[times](llvm#85693)
[before](llvm#96802). There are a
couple of fixes that the testing exposed, and I've abandoned the DWP
tests because I want to get those fixes finally upstreamed, as without
them DebugInfoD is less useful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants